Skip to content

Conversation

@Blendify
Copy link
Member

@Blendify Blendify commented Mar 31, 2017

There will be changes down in master after this is accepted but for right now I left them out to avoid merge conflicts.

@agjohnson agjohnson added the PR: work in progress Pull request is not ready for full review label Mar 31, 2017
@Blendify
Copy link
Member Author

See for example https://borgbackup.readthedocs.io/en/stable/

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, but needs some kind of user facing documentation so that people know it's an option. We should really have a "configuration options" doc.

@Blendify
Copy link
Member Author

I started working a new docs branch but for now, I am going to add this to the readme

'prev_next_buttons_location': bottom,
'style_external_links': False,
# CSS Options
'nav_search_background_color': #2980B9,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the # will need to be escaped somehow?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's just a string in the conf.py directory. Wouldn't it simply be quoted?

@davidfischer
Copy link
Contributor

I'm having some issues testing this and perhaps you could point me in the right direction.

Firstly, I don't think you can simply add nav_search_background_color to html_theme_options dict unless it is mentioned under [options] of theme.conf, right? At least I couldn't get it working without that. I don't see any mention of [colors]. Here's the relevant docs.

Based on what I can see since most of the actual overrides are to the saas files wouldn't somebody wanting to use this have to rebuild the theme (with Grunt rather than just making an entry in html_theme_options?

@Blendify
Copy link
Member Author

Blendify commented Mar 7, 2018

After rethinking this I do not like this hack of passing strings around our sass. I think we should remove this sass styling and move it over the a "configurable' CSS file. Does this sound like a better idea?

@agjohnson agjohnson added this to the v0.4.0 milestone Mar 29, 2018
@davidfischer
Copy link
Contributor

Does this sound like a better idea?

To me, yes. I don't think things that can be configured easily with an additional CSS file should be a theme option.

@agjohnson agjohnson modified the milestones: v0.4.0, 0.5 Jun 7, 2018
@agjohnson agjohnson modified the milestones: 0.5, 0.6 May 3, 2020
@Blendify
Copy link
Member Author

This is now possible with https://sphinx-rtd-theme.readthedocs.io/en/stable/configuring.html#confval-style_nav_header_background

In the future, I would like to revisit this to see if its possible to make theme variables affect sass directly.

@Blendify Blendify closed this Mar 13, 2021
@Blendify Blendify deleted the nav-search-background-color branch March 13, 2021 19:22
@Blendify Blendify modified the milestones: 1.1, 1.0 Apr 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: work in progress Pull request is not ready for full review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants